Skip to content

Fixes #2443 Restore simulation association on PI snapshot import#2444

Merged
rwmcintosh merged 2 commits into
developfrom
fixes-2443-pi-simulation-not-set-on-snapshot-import
Jul 21, 2026
Merged

Fixes #2443 Restore simulation association on PI snapshot import#2444
rwmcintosh merged 2 commits into
developfrom
fixes-2443-pi-simulation-not-set-on-snapshot-import

Conversation

@rwmcintosh

@rwmcintosh rwmcintosh commented Jul 20, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Fixes #2443

Loading a project snapshot containing a parameter identification dropped the PI's simulation association (and threw a NullReferenceException).

Two causes, both fixed here:

  • ProjectMapper mapped parameter identifications before simulations were added to the project, so name lookups failed. PIs are now mapped after simulations load, matching PK-Sim.
  • MoBiProject.All<T>() only enumerated building blocks, but simulations live in a separate list. The Core PI mapper resolves the PI's Simulations via project.All<ISimulation>(), which was always empty in MoBi. All<T>() now includes simulations.

Summary by CodeRabbit

  • Bug Fixes
    • Improved project item retrieval so simulations are included consistently alongside other project components.
    • Adjusted project loading so parameter identifications are restored after simulation setup, improving snapshot loading consistency.

@rwmcintosh
Fixes #2443 Restore simulation association on PI snapshot import
ae0594a 
Parameter identifications were mapped before simulations were added to
the project, so simulation lookups failed and the PI lost its simulation
(and threw an NRE resolving linked parameters). Map PIs after simulations
are loaded, matching PK-Sim.

Also include simulations in MoBiProject.All<T>() (they live in a separate
list from building blocks) so the Core PI mapper's project.All<ISimulation>()
lookup resolves them.
@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@rwmcintosh, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4c4034c7-fc0c-4bf4-9781-1388c38a8ec5

📥 Commits

Reviewing files that changed from the base of the PR and between ae0594a and 7e0c1bb.

📒 Files selected for processing (2)
  • tests/MoBi.Tests/IntegrationTests/Snapshots/ParameterIdentificationSnapshotSpecs.cs
  • tests/MoBi.Tests/TestFiles/snapshot_no_pksim_modules.json
📝 Walkthrough

Walkthrough

MoBiProject now includes simulations in generic object enumeration. Snapshot mapping loads parameter identifications after simulation setup.

Changes

Project loading behavior

Layer / File(s) Summary
Simulation enumeration
src/MoBi.Core/Domain/Model/MoBiProject.cs		 get<T>() searches building blocks and simulations, and All() delegates directly to All<IObjectBase>().
Snapshot parameter loading
src/MoBi.Core/Snapshots/Mappers/ProjectMapper.cs		 Parameter identifications are mapped later in mapToModel, after the simulation-loading/setup block.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • Issue 2443 — Moves parameter-identification mapping until after simulations are loaded.

Poem

I’m a bunny with a project to hop,
Simulations now join the object crop.
Parameters wait till simulations appear,
Then load in their proper place here.
Fluffy logic, tidy flow—
A cheerful merge from nose to toe!

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: restoring simulation associations during parameter identification snapshot import.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fixes-2443-pi-simulation-not-set-on-snapshot-import

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rwmcintosh rwmcintosh self-assigned this Jul 20, 2026
@rwmcintosh
Add regression test for PI simulation association on snapshot import
7e0c1bb 
Link the existing parameter identification in the test fixture to its
simulation (additive change, preserving the existing charts/curves) and
add an integration spec asserting the simulation and identification
parameter survive a snapshot load.
@rwmcintosh
rwmcintosh requested review from Yuri05 and msevestre July 21, 2026 12:41
@rwmcintosh

rwmcintosh commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai when you cannot review a PR due to rate limit that should not mean the PR has a failing check. We have other reviewers.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@rwmcintosh You’re right—an unavailable review caused by rate limiting should not be treated as a failing quality signal for the PR. I’ve noted that distinction for future reviews.

🐇💭

✏️ Learnings added 
Learnt from: rwmcintosh
URL: https://github.com/Open-Systems-Pharmacology/MoBi/pull/2444

Timestamp: 2026-07-21T16:47:25.680Z
Learning: For the Open-Systems-Pharmacology/MoBi repository, a CodeRabbit review that cannot run because of rate limiting should not be treated or reported as a failing PR check; other reviewers may still review the pull request.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@rwmcintosh
rwmcintosh merged commit 2707898 into develop Jul 21, 2026
5 of 6 checks passed
@rwmcintosh
rwmcintosh deleted the fixes-2443-pi-simulation-not-set-on-snapshot-import branch July 21, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yuri05 Yuri05 Yuri05 approved these changes
@msevestre msevestre Awaiting requested review from msevestre

Assignees

@rwmcintosh rwmcintosh

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Loading snapshot with a parameter identification throws NullReferenceException

2 participants

@rwmcintosh @Yuri05